Migrate to pnpm
#3889
Migrate to pnpm
#3889
Conversation
|
Codecov Report
@@ Coverage Diff @@
## main #3889 +/- ##
==========================================
+ Coverage 75.12% 75.14% +0.01%
==========================================
Files 195 195
Lines 11482 11485 +3
Branches 3033 3034 +1
==========================================
+ Hits 8626 8630 +4
+ Misses 2856 2855 -1
|
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6098052926/npm-package-wrangler-3889You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6098052926/npm-package-wrangler-3889Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6098052926/npm-package-wrangler-3889 dev path/to/script.jsAdditional artifacts:npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/6098052926/npm-package-cloudflare-pages-shared-3889Note that these links will no longer work once the GitHub Actions artifact expires.
| Please ensure constraints are pinned, and |
|
If you didn't do it, I was going to. It works better for Monorepo's and has other benefits! Looks like Windows making things difficult 😞 |
Co-authored-by: Pete Bacon Darwin <[email protected]>
|
Overall this looks good to me, but there's a bit of an issue with the c3 e2e tests. This change causes tests to use The package detection code here falls back to npm as the default, which will no longer be the case when this lands. This change should fix it. |
What this PR solves / how to test:
This is a big PR, but it's mostly because of the shift from
npm'spackage-lock.jsonto thepnpmsystem. The rest of the PR deals with "phantom" dependencies that various parts of the codebase had—making sure those relationships were spelled out more explicitly.Is
pnpmworth it, you might ask? I think so. Beyond the raw speed improvements (hello 11s package install times in CI!), it also provides much better dependency isolation, which is starting to become a major problem for us as a large-ish monorepo. An upcoming PR relies on using a different version of React (18) than that used in Wrangler (17), which was tricky (read, "I gave up and switched topnpminstead") to get working with the npm workspaces install algorithm (React itself was correctly hoisted to the package scope, but other dependencies weren't, leading to components from packages importing React 17 (from Wrangler), not React 18 (from the new package) as they should have done).pnpmprovides a lot more customisability around hoisting, and also has better defaults (hoisting dependencies to the package scope by default).This should change nothing for consumers of the Wrangler package (it'll still be usable from
npm,yarn, andpnpm, as it is currently), but it'll change the workflow for contributors to Wrangler, who will need to switch to usingpnpmfor all installs. To that end, I've made updates to the README to usepnpmcommands (and to specifically call out how to installpnpmfor those who might not already have it installed).In lieu of updating the
wranglerjs-compat-webpack-pluginpackage to work withpnpm, the package has been removed. After team discussion, this package will be deprecated, but the source will continue to be available in the git history, or on thev2-maintenancebranchAuthor has included the following, where applicable:
[ ] Tests[ ] Changeset (Changeset guidelines)Reviewer is to perform the following, as applicable:
Note for PR author:
We want to celebrate and highlight awesome PR review! If you think this PR received a particularly high-caliber review, please assign it the label
highlight pr reviewso future reviewers can take inspiration and learn from it.